Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[USERGUIDE] LLM Hyperparameter Optimization API #3952

Open
wants to merge 48 commits into
base: master
Choose a base branch
from
Open

[USERGUIDE] LLM Hyperparameter Optimization API #3952

wants to merge 48 commits into from

Conversation

mahdikhashan
Copy link
Contributor

ref: #3951

@google-oss-prow Google OSS Prow
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign gaocegege for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow Google OSS Prow
Copy link

Hi @mahdikhashan. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@mahdikhashan
Copy link
Contributor Author

mahdikhashan commented Jan 7, 2025 •
edited
Loading

hi @andreyvelich , shall i keep it under user-guides/hp-tuning/?

@andreyvelich
Copy link
Member

Sure, I think we can create a new page for this feature.
FYI, please follow the contribution guide to sign the commits: https://www.kubeflow.org/docs/about/contributing/#getting-started
cc @helenxie-bit

@andreyvelich
Copy link
Member

Part of: kubeflow/katib#2339

Arhell
Arhell reviewed Jan 8, 2025
View reviewed changes
Copy link
Member

@Arhell Arhell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ok-to-test

@mahdikhashan
add base md
181719d 
Signed-off-by: mahdikhashan <[email protected]>
@mahdikhashan
update title and description
aa3b2be 
Signed-off-by: mahdikhashan <[email protected]>
@mahdikhashan mahdikhashan marked this pull request as ready for review January 11, 2025 18:26
@google-oss-prow google-oss-prow bot requested a review from sperlingxx January 11, 2025 18:26
@mahdikhashan
add draft code
36f2c1e 
Signed-off-by: mahdikhashan <[email protected]>
@mahdikhashan
add prerequisites
b7120e0 
Signed-off-by: mahdikhashan <[email protected]>
@mahdikhashan
add huggingface api details,s3 api, update example
182b493 
Signed-off-by: mahdikhashan <[email protected]>
@mahdikhashan
remove redundant text
21646a8 
Signed-off-by: mahdikhashan <[email protected]>
@mahdikhashan
add HuggingFaceTrainerParams description
cc71dee 
Signed-off-by: mahdikhashan <[email protected]>
@mahdikhashan
update prerequisites
56e4e53 
Signed-off-by: mahdikhashan <[email protected]>
@mahdikhashan
update code example
6f36f1e 
Signed-off-by: mahdikhashan <[email protected]>
helenxie-bit
helenxie-bit reviewed Jan 17, 2025
View reviewed changes
Copy link
Contributor

@helenxie-bit helenxie-bit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for incorporating my suggestions! Here are some further comments.

content/en/docs/components/katib/user-guides/hp-tuning/llm-hp-optimization.md Outdated Show resolved Hide resolved
content/en/docs/components/katib/user-guides/hp-tuning/llm-hp-optimization.md Outdated Show resolved Hide resolved
content/en/docs/components/katib/user-guides/hp-tuning/llm-hp-optimization.md Outdated Show resolved Hide resolved
content/en/docs/components/katib/user-guides/hp-tuning/llm-hp-optimization.md Outdated Show resolved Hide resolved
content/en/docs/components/katib/user-guides/hp-tuning/llm-hp-optimization.md Outdated Show resolved Hide resolved
@mahdikhashan
Copy link
Contributor Author

thanks for your comments @helenxie-bit , i resolved all of them - please let me know of your current opinion.

helenxie-bit
helenxie-bit reviewed Jan 17, 2025
View reviewed changes
Copy link
Contributor

@helenxie-bit helenxie-bit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically LGTM! I left a few minor comments for your consideration.

content/en/docs/components/katib/user-guides/hp-tuning/llm-hp-optimization.md Outdated Show resolved Hide resolved
content/en/docs/components/katib/user-guides/hp-tuning/llm-hp-optimization.md Outdated Show resolved Hide resolved
content/en/docs/components/katib/user-guides/hp-tuning/llm-hp-optimization.md Outdated Show resolved Hide resolved
@mahdikhashan
Copy link
Contributor Author

@helenxie-bit thanks for your patience and help with this issue. i addressed recent comments.

@mahdikhashan
improve prerequisites
e0ef337 
Signed-off-by: mahdikhashan <[email protected]>
@helenxie-bit
Copy link
Contributor

Thanks for the great contribution!

/lgtm

/cc @kubeflow/wg-automl-leads @andreyvelich @Arhell

@google-oss-prow google-oss-prow bot requested a review from Arhell January 18, 2025 21:23
@google-oss-prow Google OSS Prow
Copy link

@helenxie-bit: GitHub didn't allow me to request PR reviews from the following users: kubeflow/wg-automl-leads.

Note that only kubeflow members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Thanks for the great contribution!

/lgtm

/cc @kubeflow/wg-automl-leads @andreyvelich @Arhell

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@google-oss-prow google-oss-prow bot added the lgtm label Jan 18, 2025
andreyvelich
andreyvelich reviewed Jan 23, 2025
View reviewed changes
Copy link
Member

@andreyvelich andreyvelich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@varodrig @pdarshane @rimolive @hbelmiro Please help with reviewing this documentation page for LLM HP Tuning API.

helenxie-bit
helenxie-bit reviewed Jan 24, 2025
View reviewed changes
content/en/docs/components/katib/user-guides/hp-tuning/llm-hp-optimization.md
You need to install the following Katib components to run code in this guide:

- Katib control plane – [install](/docs/components/katib/installation/#installing-control-plane)
- Training Operator control plane – [install](/docs/components/training/installation/#installing-the-control-plane)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to discussion here, can you move "Training Operator control plane" in front of "Katib control plane"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And it would be better to add a small note about it in this section :) For example:

Note: Please ensure that the Training Operator control plane is installed before the Katib control plane. This ensures the correct namespace labels are applied and enables the PyTorchJob CRD for use in Katib.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@helenxie-bit helenxie-bit helenxie-bit left review comments

@andreyvelich andreyvelich andreyvelich left review comments

@johnugeorge johnugeorge Awaiting requested review from johnugeorge

@sperlingxx sperlingxx Awaiting requested review from sperlingxx

@Arhell Arhell Awaiting requested review from Arhell

Assignees

@helenxie-bit helenxie-bit

Labels
lgtm ok-to-test size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mahdikhashan @andreyvelich @helenxie-bit @Arhell